Add CI workflows #357
Add CI workflows #357
Conversation
|
/run standalone |
I was hoping that this would work for this PR, as the CI worked but I guess I was mistaken. As a result, there is not easy way for me to check how this works. |
|
In CI results of this PR I see multiple mentions of |
|
In (SegmentLinking/TrackLooper-actions/cmssw/action.yml](https://github.com/SegmentLinking/TrackLooper-actions/blob/main/cmssw/action.yml), I see a mention of your username, @ariostas. Is this going to work for others? Does it make a real difference? |
|
All in all, does it make sense to just merge this PR to be able to test it in action? I couldn't find any smoking gun issues, apart from the minor comments above, and I don't think that anything would break in any case. |
|
@VourMa thanks for your comments. Unfortunately, as you said, we can't self-test this PR since this action runs only when it's in the main branch. The errors you're seeing are expected. I'm running the setup script to set some environment variables, but since the linux version doesn't match what cmssw expects there are some issues, but it doesn't affect anything that we need. As for my username, it appears because I'm storing the docker images in my personal docker hub account. They have restricted a lot when it comes to free organization accounts, but if you prefer we can create some personal account for LST and store it there. I added instructions in the docker folder of the actions repo for how to do this if we need to move the images somewhere else later on. If you want to try things you can look at the repo in the SegmentLinkinTests organization. But I think it makes sense to just merge it. If something is broken most likely we'll be able to fix it from the other repo. But I tested things, so everything should be good to go. |
|
Thanks for the reply!
I think this is ok as long as it works for others. The instructions are useful for the long-term, thank you!
If I merge, would I be able to run the |
|
@VourMa you can run it with your PR after merging. You don't need to rebase. However, the formatting won't be checked unless you push something new. Maybe let's make sure that the forgetting is fine by manually running clang-format. |
Should I just push a "dummy" commit to PR #358 to trigger the tests? Or should I run something manually on my terminal? |
|
|
||
| ## Static code checks | ||
|
|
||
| The workflow that runs static code checks is based on the clang-format and clang-tidy requirements from CMSSW. They are run automatically for commits in pull-requests, and should be roughly equivalent to running `scram build code-checks` and `scram build code-format`. They are fairly lenient, checking only the lines that were changed without making any modifications. The aim is to make sure that the code is compliant for the CMSSW integration. More information can be found [here](https://cms-sw.github.io/PRWorkflow.html). As work towards the full integration with CMSSW progresses, we will tune these checks to become more stringent and closer to what is done during the CMSSW CI checks. |
There was a problem hiding this comment.
is there a way to run the code checks locally without the CI?
There was a problem hiding this comment.
If you want to check formatting you can run clang-format -i -n SDL/*.h SDL/*.cc. To apply the changed you simply remove the -n flag.
To check with clang-tidy is more annoying. It should be something like this.
INCLUDE_FLAGS="-I$TRACKLOOPERDIR -I$BOOST_ROOT/include -I$ALPAKA_ROOT/include -I$CUDA_HOME/include -I$ROOT_ROOT/include -I$CMSSW_BASE/src"
INCLUDE_FLAGS="$INCLUDE_FLAGS -I/cvmfs/cms.cern.ch/el8_amd64_gcc11/external/gcc/11.4.1-30ebdc301ebd200f2ae0e3d880258e65/include/c++/11.4.1"
INCLUDE_FLAGS="$INCLUDE_FLAGS -I/cvmfs/cms.cern.ch/el8_amd64_gcc11/external/gcc/11.4.1-30ebdc301ebd200f2ae0e3d880258e65/include/c++/11.4.1/x86_64-redhat-linux-gnu/"
INCLUDE_FLAGS="$INCLUDE_FLAGS -I/cvmfs/cms.cern.ch/el8_amd64_gcc11/external/gcc/11.4.1-30ebdc301ebd200f2ae0e3d880258e65/lib/gcc/x86_64-redhat-linux-gnu/11.4.1/include"
clang-tidy SDL/*.h SDL/*.cc -- --language=c++ -std=c++17 -DALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLED $INCLUDE_FLAGS
This PR implements CI workflows to perform static checks with
clang-formatandclang-tidy, as well as to perform test runs of the code in standalone and cmssw setups.The static checks are performed automatically for all commits to PRs. The CI doesn't fix anything, but it gives warnings for formatting and other potential issues. I couldn't get
clang-tidyto work perfectly, but it works well enough for now. My plan is that when in a few months the CMSSW integration has progressed enough, I'll update the workflow to usescram b code-formatandscram b code-checksto more accurately mirror what is done by the cmsbuild bot.The testing workflows are triggered by leaving comments on PRs containing
/run standaloneand/or/run cmssw. The latter command accepts an optional argument to specify the branch of the CMSSW fork that should be used.I have documented everything more extensively in the
.github/workflows/README.mdfile and in the SegmentLinking/TrackLooper-actions repository.